[mypyc] Move API table definitions to .c files#21183
[mypyc] Move API table definitions to .c files#21183p-sawicki wants to merge 10 commits intopython:masterfrom
Conversation
mypyc/lib-rt/static_data.c
Outdated
|
|
||
| #ifdef MYPYC_EXPERIMENTAL | ||
|
|
||
| void *LibRTStrings_API[LIBRT_STRINGS_API_LEN] = {0}; |
There was a problem hiding this comment.
Now all the API tables will always be included in each build, which increases the memory usage of each module a little. It's not a big deal right now, but this is not scalable if we add many librt submodules in the future. Could we instead have librt_vecs_static.c etc. and add them as dependencies?
There was a problem hiding this comment.
yeah that's a good point. i've moved the definitions to separate files and extended the dependency handling to compile/include them when needed.
for more information, see https://pre-commit.ci
aa54c7e to
edf9c52
Compare
mypyc/build.py
Outdated
| ), | ||
| ModDesc("librt.time", ["time/librt_time.c"], ["time/librt_time.h"], []), | ||
| ModDesc( | ||
| "librt.time", ["time/librt_time_static.c", "time/librt_time.c"], ["time/librt_time.h"], [] |
There was a problem hiding this comment.
Do we need to include the _static.c files when building librt submodules? They should be only used by mypyc compiled code, I think.
|
|
||
| #define LIBRT_TIME_ABI_VERSION 1 | ||
| #define LIBRT_TIME_API_VERSION 1 | ||
| #define LIBRT_TIME_API_LEN 3 |
There was a problem hiding this comment.
These definitions duplicate the ones in librt_time.h.
|
|
||
| #define LibRTTime_ABIVersion (*(int (*)(void)) LibRTTime_API[0]) | ||
| #define LibRTTime_APIVersion (*(int (*)(void)) LibRTTime_API[1]) | ||
| #define LibRTTime_time (*(double (*)(void)) LibRTTime_API[2]) |
There was a problem hiding this comment.
These duplicate definitions in librt_time_api.h.
lib-rt API tables are defined as static variables in lib-rt headers. This means that each translation unit gets its own independent instance of these variables.
That becomes a problem in multi-file compilation mode when using
BytesWriter.write, as this function is translated toCPyBytesWriter_Writeby mypyc which is defined inbytes_writer_extra_ops.c. With multi-file compilation,bytes_writer_extra_ops.cis compiled as its own translation unit and gets linked with the C extension compiled from python files.The C extension TU copies the
librt.stringscapsule contents into the global table but because it's static this is not visible in the table in thebytes_writer_extra_ops.cTU, which stays zero-initialized. This results in a seg fault with the following call chain:CPyBytesWriter_Write->CPyBytesWriter_EnsureSize->LibRTStrings_ByteWriter_grow_buffer_internal->LibRTStrings_API[5]-> oops all zerosTo fix this, declare the tables as
externand define them in .c files so there's only one global version of each variable. There's one new .c file for each lib-rt module that is compiled or included when mypyc detects a dependency on that module.